Skip to content

Ensure FrozenEngine always applies engine-level wrapper#76100

Merged
ywelsch merged 2 commits intoelastic:masterfrom
ywelsch:fix-engine-wrapper
Aug 5, 2021
Merged

Ensure FrozenEngine always applies engine-level wrapper#76100
ywelsch merged 2 commits intoelastic:masterfrom
ywelsch:fix-engine-wrapper

Conversation

@ywelsch
Copy link
Contributor

@ywelsch ywelsch commented Aug 4, 2021

FrozenEngine does not always apply the engine-level DirectoryReader wrapper. Only affects combination of source-only snapshot repositories and shared_cache searchable snapshots. While it is technically a bug, it is such an exotic combination of things that it might not be even worth calling out. The fix is mostly here to ensure that future wrappers won't suffer from the same fate.

The test now exercises loading _seq_no (which is added by wrapper) when running AbstractSearchableSnapshotsRestTestCase.testSourceOnlyRepository.

@ywelsch ywelsch added >bug :Distributed/Engine Anything around managing Lucene and the Translog in an open shard. v8.0.0 v7.15.0 labels Aug 4, 2021
@ywelsch ywelsch requested a review from dnhatn August 4, 2021 15:01
@elasticmachine elasticmachine added the Team:Distributed Meta label for distributed team. label Aug 4, 2021
@elasticmachine
Copy link
Collaborator

elasticmachine commented Aug 4, 2021

Pinging @elastic/es-distributed (Team:Distributed)

dnhatn
dnhatn approved these changes Aug 4, 2021
View reviewed changes
Copy link
Member

@dnhatn dnhatn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left a nit in tests, LGTM.

@ywelsch
Copy link
Contributor Author

ywelsch commented Aug 4, 2021

Thanks @dnhatn

@ywelsch ywelsch merged commit 6f31abc into elastic:master Aug 5, 2021
ywelsch added a commit that referenced this pull request Aug 5, 2021
@ywelsch
Ensure FrozenEngine always applies engine-level wrapper (#76100)
90c3de8 
FrozenEngine does not always apply the engine-level DirectoryReader wrapper. Only affects combination of source-only
snapshot repositories and shared_cache searchable snapshots. While it is technically a bug, it is such an exotic
combination of things that it might not be even worth calling out. The fix is mostly here to ensure that future wrappers
won't suffer from the same fate.
ywelsch added a commit that referenced this pull request Aug 5, 2021
@ywelsch
Revert "Ensure FrozenEngine always applies engine-level wrapper (#76100…
02fbbf3 
…)"

This reverts commit 90c3de8.
ywelsch added a commit that referenced this pull request Aug 5, 2021
@ywelsch
Revert "Ensure FrozenEngine always applies engine-level wrapper (#76100…
3ba4230 
…)"

This reverts commit 6f31abc.
@ywelsch
Copy link
Contributor Author

ywelsch commented Aug 5, 2021

I've reverted this PR as it uncovered some interesting existing bugs with source-only snapshots on CI (that have nothing to do with this PR, but the PR better exercises the existing functionality).

The test exercised with this PR was using multiple shards (in contrast to SourceOnlySnapshotIT, which just uses a single shard), which led to the discovery that sorting by _seq_no when searching on a source-only snapshot (something explicitly tested) breaks in the can-match phase (which only comes into play when there are multiple shards) as the wrapper does not properly implement min-max retrieval. This was broken when #49092 was added, but the source-only snapshot tests were not general enough to catch it.

Failed to execute phase [can_match], all shards failed
; shardFailures {[OrqVOyPJSL2kjTDOEVOATA][test-idx][0]: org.elasticsearch.transport.RemoteTransportException: [node_s3][127.0.0.1:49760][indices:data/read/search[can_match]]
Caused by: org.elasticsearch.common.io.stream.NotSerializableExceptionWrapper: unsupported_operation_exception: _source only indices can't be searched or filtered
	at org.elasticsearch.snapshots.sourceonly.SeqIdGeneratingFilterReader$SeqIdGeneratingSubReaderWrapper$1.getPointValues(SeqIdGeneratingFilterReader.java:164)
	at org.apache.lucene.index.FilterLeafReader.getPointValues(FilterLeafReader.java:330)
	at org.apache.lucene.index.PointValues.size(PointValues.java:102)
	at org.elasticsearch.search.sort.FieldSortBuilder.extractNumericMinAndMax(FieldSortBuilder.java:568)
	at org.elasticsearch.search.sort.FieldSortBuilder.getMinMaxOrNull(FieldSortBuilder.java:548)
	at org.elasticsearch.search.SearchService.canMatch(SearchService.java:1279)
	at org.elasticsearch.search.SearchService.canMatch(SearchService.java:1231)
	at org.elasticsearch.search.SearchService.canMatch(SearchService.java:1219)
	at org.elasticsearch.action.search.SearchTransportService.lambda$registerRequestHandler$13(SearchTransportService.java:364)
	at org.elasticsearch.transport.RequestHandlerRegistry.processMessageReceived(RequestHandlerRegistry.java:61)
	at org.elasticsearch.transport.InboundHandler.handleRequest(InboundHandler.java:201)
	at org.elasticsearch.transport.InboundHandler.messageReceived(InboundHandler.java:100)
	at org.elasticsearch.transport.InboundHandler.inboundMessage(InboundHandler.java:82)
	at org.elasticsearch.transport.TcpTransport.inboundMessage(TcpTransport.java:695)
	at org.elasticsearch.transport.InboundPipeline.forwardFragments(InboundPipeline.java:129)
	at org.elasticsearch.transport.InboundPipeline.doHandleBytes(InboundPipeline.java:104)
	at org.elasticsearch.transport.InboundPipeline.handleBytes(InboundPipeline.java:69)
	at org.elasticsearch.transport.nio.MockNioTransport$MockTcpReadWriteHandler.consumeReads(MockNioTransport.java:312)
	at org.elasticsearch.nio.SocketChannelContext.handleReadBytes(SocketChannelContext.java:217)
	at org.elasticsearch.nio.BytesChannelContext.read(BytesChannelContext.java:29)
	at org.elasticsearch.nio.EventHandler.handleRead(EventHandler.java:128)
	at org.elasticsearch.transport.nio.TestEventHandler.handleRead(TestEventHandler.java:140)
	at org.elasticsearch.nio.NioSelector.handleRead(NioSelector.java:409)
	at org.elasticsearch.nio.NioSelector.processKey(NioSelector.java:235)
	at org.elasticsearch.nio.NioSelector.singleLoop(NioSelector.java:163)
	at org.elasticsearch.nio.NioSelector.runLoop(NioSelector.java:120)
	at java.lang.Thread.run(Thread.java:831)

@fcofdez fcofdez mentioned this pull request Aug 5, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dnhatn dnhatn dnhatn approved these changes

Assignees

No one assigned

Labels

>bug :Distributed/Engine Anything around managing Lucene and the Translog in an open shard. Team:Distributed Meta label for distributed team.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ywelsch @elasticmachine @dnhatn